Skip to content

CM launcher memory optimization - #507

Merged
al1img merged 8 commits into
aosedge:feature_unificationfrom
mykola-kobets-epam:integration-fixes
Feb 10, 2026
Merged

CM launcher memory optimization#507
al1img merged 8 commits into
aosedge:feature_unificationfrom
mykola-kobets-epam:integration-fixes

Conversation

@mykola-kobets-epam

Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the CM launcher’s scheduling/status flow to reduce memory usage by removing per-node stored instance lists and instead deriving start/stop deltas from centrally tracked running statuses, while also persisting SM-facing aos::InstanceInfo inside each Instance.

Changes:

  • Move SM instance data (aos::InstanceInfo) into Instance (mSMInfo) and adjust scheduling/network setup to populate it.
  • Rework NodeManager/Node send/resend logic to operate on provided “scheduled instances” + “running statuses” rather than node-local cached instance arrays; add a shared node allocator.
  • Update launcher/balancer/instance manager APIs to pass around explicit instance collections and track running statuses in InstanceManager; update tests accordingly (manifest digest/version propagation).

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/core/cm/launcher/tests/stubs/instancerunnerstub.hpp Test stub now propagates manifest digest/version into reported statuses.
src/core/cm/launcher/tests/launcher.cpp Updates tests/helpers to compute and assert manifest digests; adjusts runner init and expected status notifications.
src/core/cm/launcher/nodemanager.hpp Renames/repurposes APIs (load SM data, notify node status received) and changes send/resend signatures; adds shared allocator.
src/core/cm/launcher/nodemanager.cpp Implements the new node init + send/resend + load-SM-data flow using the new APIs/allocator.
src/core/cm/launcher/nodeitf.hpp Removes ScheduleInstance from the node interface.
src/core/cm/launcher/node.hpp Changes init signature to accept allocator; removes per-node stored instance arrays; updates send/resend signatures.
src/core/cm/launcher/node.cpp Implements delta computation from running statuses + scheduled instances; adds filtering helper; simplifies stop conversion.
src/core/cm/launcher/launcher.hpp Replaces “schedule” helpers with “create requested instances” helpers; increases launcher allocator sizing.
src/core/cm/launcher/launcher.cpp Adjusts lock ordering, instance creation flow, resend calls, and startup loading of SM data for stored active instances.
src/core/cm/launcher/instancemanager.hpp Adds running-instances tracking + new create/schedule APIs and helpers.
src/core/cm/launcher/instancemanager.cpp Implements running-status tracking and new create/schedule flow; updates storage-load behavior.
src/core/cm/launcher/instance.hpp Adds GetSMInfo(), embeds mSMInfo, and splits network param ops into Instance methods; updates schedule signature.
src/core/cm/launcher/instance.cpp Populates mSMInfo during schedule; moves network param setup/removal into instance implementations.
src/core/cm/launcher/balancer.hpp Changes RunInstances signature to accept explicit instance list; renames load method; updates balancing helpers.
src/core/cm/launcher/balancer.cpp Wires new instance-list-driven balancing, new scheduling API, and updated network param calls.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}

instance = FindCachedInstance(id);
if (instance) {

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FindReadyInstance() returns an instance from mCachedInstances without removing it from the cache. Later, when that same SharedPtr is scheduled and SubmitScheduledInstances() promotes mScheduledInstances to mActiveInstances, the instance can end up in both mActiveInstances and mCachedInstances. This can lead to incorrect behavior (e.g., RemoveOutdatedInstances() deleting an active instance via the cached list) and duplicated bookkeeping. Consider removing the instance from mCachedInstances when it is selected for scheduling (e.g., in FindReadyInstance() when it finds a cached instance, or in ScheduleInstance(...) before adding to mScheduledInstances).

Suggested change
if (instance) {
if (instance) {
// The instance is transitioning out of the cache; remove it from mCachedInstances
for (auto it = mCachedInstances.begin(); it != mCachedInstances.end(); ++it) {
if (*it == instance) {
mCachedInstances.erase(it);
break;
}
}

Copilot uses AI. Check for mistakes.

@mykola-kobets-epam mykola-kobets-epam Feb 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed scheduled instance from cached in SubmitScheduledInstances

RetWithError<SharedPtr<Instance>> InstanceManager::CreateInstance(
const InstanceIdent& id, const RunInstanceRequest& request)
{
auto instance = FindReadyInstance(id);

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CreateInstance() calls FindReadyInstance(id) twice and also leaves an unused local (auto instance = FindReadyInstance(id);). This is redundant and may trigger unused-variable warnings depending on build flags. Consider removing the first assignment and keeping a single lookup (e.g., the init-statement form in the if).

Suggested change
auto instance = FindReadyInstance(id);

Copilot uses AI. Check for mistakes.

@mykola-kobets-epam mykola-kobets-epam Feb 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed duplicate

Comment thread src/core/cm/launcher/balancer.cpp Outdated
Comment on lines 136 to 137
auto nodes = MakeUnique<StaticArray<Node*, cMaxNumNodes>>(&mAllocator);
auto instanceInfo = MakeUnique<aos::InstanceInfo>(&mAllocator);

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Balancer::ScheduleInstance() allocates instanceInfo but no longer uses it after the refactor to InstanceManager::ScheduleInstance(...). This can produce an unused-variable warning (and fail the build if warnings are treated as errors). Consider removing instanceInfo (and any related allocator sizing if it was only needed for that).

Suggested change
auto nodes = MakeUnique<StaticArray<Node*, cMaxNumNodes>>(&mAllocator);
auto instanceInfo = MakeUnique<aos::InstanceInfo>(&mAllocator);
auto nodes = MakeUnique<StaticArray<Node*, cMaxNumNodes>>(&mAllocator);

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed unused instanceInfo

Comment thread src/core/cm/launcher/balancer.cpp Outdated
Error Balancer::PerformPolicyBalancing(Array<SharedPtr<Instance>>& instances)
{
auto imageIndex = MakeUnique<oci::ImageIndex>(&mAllocator);
auto instanceInfo = MakeUnique<aos::InstanceInfo>(&mAllocator);

Copilot AI Feb 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PerformPolicyBalancing() still allocates instanceInfo but doesn't use it anymore. This can trigger unused-variable warnings and should be removed.

Suggested change
auto instanceInfo = MakeUnique<aos::InstanceInfo>(&mAllocator);

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed unused instanceInfo


private:
static constexpr auto cStatusUpdateTimeout = Time::cMinutes * 10;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove empty line and line 145

return AOS_ERROR_WRAP(err);
}

auto loadErr

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (auto err =

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer "out of if clause" initialization for long expressions.

if (!isStashed) {
if (auto err = instance->Cache(); !err.IsNone()) {
return AOS_ERROR_WRAP(err);
const auto& id = instance->GetInfo().mInstanceIdent;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need an intermediate var here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use aliases to shorten expressions

{
if (auto err = instance->Cache(true); !err.IsNone()) {
return AOS_ERROR_WRAP(err);
const auto& id = instance->GetInfo().mInstanceIdent;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use aliases to shorten expressions

Comment thread src/core/cm/launcher/instance.hpp Outdated
virtual Error Schedule(NodeItf& node, const String& runtimeID) = 0;

/**
* Setup network parameters.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setups

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment thread src/core/cm/launcher/instance.hpp Outdated
virtual Error PrepareNetworkParams(bool onlyExposedPorts) = 0;

/**
* Remove network parameters.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removes

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment thread src/core/cm/launcher/instance.hpp Outdated
Error Schedule(NodeItf& node, const String& runtimeID) override;

/**
* Prepare network parameters.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prepares

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment thread src/core/cm/launcher/instance.hpp Outdated
Error PrepareNetworkParams(bool onlyExposedPorts) override;

/**
* Remove network parameters.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removes

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment thread src/core/cm/launcher/instance.hpp Outdated
Error Schedule(NodeItf& node, const String& runtimeID) override;

/**
* Prepare network parameters.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prepares

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment thread src/core/cm/launcher/instance.hpp Outdated
Error PrepareNetworkParams(bool onlyExposedPorts) override;

/**
* Remove network parameters.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removes

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

{
assert(mIt != mEnd);

Iterator tmp = *this;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i suppose the beheviour shoud be same as in operator++()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a post increment operator i++,
returns incremented copy

const InstanceIdent& id, const RunInstanceRequest& request)
{
auto instance = FindReadyInstance(id);
if (auto instance = FindReadyInstance(id); instance) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why we two times call FindReadyInstance ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that's a mistake, removed duplicate

@codecov

codecov Bot commented Feb 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.41722% with 38 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.19%. Comparing base (e23272c) to head (628ce86).
⚠️ Report is 11 commits behind head on feature_unification.

Files with missing lines Patch % Lines
src/core/cm/launcher/launcher.cpp 72.54% 14 Missing ⚠️
src/core/cm/launcher/instancemanager.cpp 80.00% 11 Missing ⚠️
src/core/cm/launcher/balancer.cpp 85.71% 5 Missing ⚠️
src/core/cm/launcher/instance.cpp 93.47% 3 Missing ⚠️
src/core/cm/launcher/node.cpp 95.00% 3 Missing ⚠️
src/core/cm/launcher/nodemanager.cpp 84.61% 2 Missing ⚠️
Additional details and impacted files
@@                   Coverage Diff                   @@
##           feature_unification     #507      +/-   ##
=======================================================
- Coverage                85.20%   85.19%   -0.02%     
=======================================================
  Files                      308      308              
  Lines                    27154    27181      +27     
  Branches                  3669     3663       -6     
=======================================================
+ Hits                     23136    23156      +20     
- Misses                    4018     4025       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@al1img al1img left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>

@MykolaSuperman MykolaSuperman left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-by: Mykola Solianko <mykola_solianko@epam.com>

Signed-off-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykola Solianko <mykola_solianko@epam.com>
Signed-off-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykola Solianko <mykola_solianko@epam.com>
Signed-off-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykola Solianko <mykola_solianko@epam.com>
Signed-off-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykola Solianko <mykola_solianko@epam.com>
Signed-off-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykola Solianko <mykola_solianko@epam.com>
Signed-off-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykola Solianko <mykola_solianko@epam.com>
Signed-off-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykola Solianko <mykola_solianko@epam.com>
Signed-off-by: Mykola Kobets <mykola_kobets@epam.com>
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Reviewed-by: Mykola Solianko <mykola_solianko@epam.com>
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
49.7% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@al1img
al1img merged commit 3bfeb08 into aosedge:feature_unification Feb 10, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants